Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Isolate cookiebar module JS #119

Merged
merged 12 commits into from
May 9, 2024
Merged

Isolate cookiebar module JS #119

merged 12 commits into from
May 9, 2024

Conversation

sergei-maertens
Copy link
Collaborator

@sergei-maertens sergei-maertens commented May 2, 2024

This puts the source code for the cookiebar module in its own source directory, allowing us to use Typescript to compile it down to JS.

Advantages:

  • We can still build it and ship it with django staticfiles, similar to the 0.5.0b0 approach
  • Additionally, we can expose the "npm package" on Github so that users can more easily include this in their own frontend pipeline and bundle it in their main entrypoint.
  • We can use typescript for better robustness/less bugs & ship type declarations for users of Typescript themselves

TODO:

  • Rewrite in TS
  • Build 'package' from TS
  • Expose package on Github (perhaps explore github packages as an alternative to npm registry?) not viable, see Need to make npm package public #120
  • Ensure that the CI pipeline builds and publishes the build artifacts (so that npm install https://... works properly) publishing can't be automated at the moment
  • Set up build tooling to bundle into static/cookie_consent/cookiebar.module.js

Copy link

codecov bot commented May 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.03%. Comparing base (175d7ba) to head (6c37cf0).
Report is 7 commits behind head on master.

❗ Current head 6c37cf0 differs from pull request most recent head 0191b88. Consider uploading reports for the commit 0191b88 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #119      +/-   ##
==========================================
- Coverage   96.08%   96.03%   -0.05%     
==========================================
  Files          12       11       -1     
  Lines         409      404       -5     
  Branches       70       52      -18     
==========================================
- Hits          393      388       -5     
  Misses         11       11              
  Partials        5        5              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Added boilerplate to create/bundle an npm package on Github, so that
the build artifact(s) can be included in the static files of the
django app for ease of use, and other projects can separately install
the package for the JS with npm to bundle the JS with their own
solution.
The added type safety helps catch bugs, and the setup is simple enough
for people not too familiar with TS to be able to contribute.

Small refactors have been done to avoid globals and localize state a
bit more, which shouldn't affect anyone.
The cookiebar module TS source is taken as input, and the
resulting bundle written to the cookie consent staticfiles
directory.
The onShow handler should be invoked after the node was inserted.

Thanks to @grrodre for contributing the patch!
The npm 'package' and the django staticfiles assets are exclusively
to be built in the CI pipeline, and not kept in version control.

This keeps a single source of truth and prevents us from forgetting to
update the artifacts, at the cost of a little extra step for local
development.
Ensure that the frontend package version is also bumped.
A package with type: module requires extensions in the imports to
fully resolve them (tested with Webpack 5). We can simply use the .js
extension even in .ts files, tsc understands it (even though this looks
really weird).
The code is different from 0.5.0 beta 0, and the next release will
be 0.6.0.
@sergei-maertens sergei-maertens marked this pull request as ready for review May 9, 2024 11:07
@sergei-maertens sergei-maertens merged commit bdc3452 into master May 9, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant